.Net: Improve input validation in OpenAPI plugin - #13962
Conversation
- Apply Uri.EscapeDataString to LLM-supplied server variable values before substitution in GetServerUrl (issue 115071) - Add ValidatePathSegments to reject dot-segments (. and ..) introduced by path parameter values, preventing path traversal (issue 115103) - Add 5 regression tests covering both fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8354c9d to
382e3e6
Compare
There was a problem hiding this comment.
Pull request overview
This PR strengthens URL construction in the OpenAPI plugin by adding validation against path traversal via dot-segments in path parameters, and by encoding server variable substitutions when building a RestApiOperation URL.
Changes:
- Encode server variable values during server URL template substitution.
- Validate built operation paths to reject dot-segments (
./..) that could trigger URI path normalization/traversal. - Add unit tests covering server variable encoding and dot-segment rejection behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs | Encodes server variable substitutions and introduces dot-segment validation for built paths. |
| dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs | Adds tests for server variable encoding/injection scenarios and dot-segment handling in path parameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 81%
✓ Correctness
The PR correctly adds path traversal protection for path parameters via
ValidatePathSegmentsand encodes server variable values withUri.EscapeDataString. However,Uri.EscapeDataStringdoes not encode dots (they are unreserved per RFC 3986), so a server variable value of exactly..or.would pass through unencoded. When theUriconstructor normalizes the resulting path (e.g.,https://example.com/v1/..→https://example.com/), this enables traversal within the server URL. This is not a regression (the vulnerability existed before) but represents a gap in the strengthened validation, since path parameters get dot-segment validation while server variables do not.
✓ Security Reliability
This PR correctly addresses server variable injection and path traversal risks. The Uri.EscapeDataString encoding for server variables prevents path/query injection, and the ValidatePathSegments check catches dot-segments (. and ..) that survive HttpUtility.UrlEncode (since dots are RFC-unreserved characters). The test coverage is thorough, including positive cases for legitimate dot-containing values. No security or reliability issues found.
✓ Test Coverage
The PR adds good test coverage for the new path traversal validation and server variable encoding. Tests cover dot-segment rejection ('..' and '.'), allowance of legitimate dots in filenames, server variable encoding, and injection prevention via UriFormatException. However, the
Uri.EscapeDataStringencoding was added to two code branches inGetServerUrl(the ArgumentName lookup branch and the variable-name fallback branch), but only the variable-name fallback branch is exercised by the new tests.
✓ Design Approach
I did not find a design-approach issue that met the evidence bar. The change is narrowly aligned with the stated goal of hardening server-variable substitution and path-parameter handling, and the added tests cover the intended hostile-input cases without revealing a repo-supported invariant or caller contract that this approach breaks.
Automated review by SergeyMenshykh's agents
- Use accumulated serverUrlString for multi-variable substitution - Add ArgumentName branch encoding test - Fix error message in ValidatePathSegments - Improve test comments for clarity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ca1dbda to
0daff71
Compare
Strengthen input validation for server variable substitution and path parameter handling in RestApiOperation.